-
Notifications
You must be signed in to change notification settings - Fork 1
runtimeにinit()を追加し、必要な時だけ初期化 #109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
my-code | 7ab2c46 | Commit Preview URL Branch Preview URL |
Nov 18 2025, 08:47 AM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the runtime initialization pattern from automatic (on component mount) to lazy initialization via a new init() method. This allows runtimes to be initialized only when needed, improving performance by avoiding unnecessary initialization of unused language runtimes.
- Added
init()method to theRuntimeContextinterface for manual initialization control - Modified worker-based runtimes (Python, Ruby, JavaScript) to use mutex-protected lazy initialization
- Updated TypeScript runtime to support lazy initialization with a guard flag
- Refactored
useRuntime()to callinit()automatically viauseEffect
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| app/terminal/worker/runtime.tsx | Converted worker initialization from automatic useEffect to manual init() method with mutex protection and early-return guard |
| app/terminal/typescript/runtime.tsx | Added init() method with initialization guard flag, removed AbortController cleanup logic |
| app/terminal/runtime.tsx | Added init field to RuntimeContext interface, modified useRuntime() to call init() via useEffect, changed switch from direct returns to break statements |
| app/terminal/page.tsx | Added manual init() call for each runtime before running tests |
| app/terminal/README.md | Documented the new init() method, its behavior, and usage patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runFiles: ( | ||
| filenames: string[], | ||
| files: Readonly<Record<string, string>> | ||
| ) => Promise<ReplOutput[]>; |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] While the reformatting of the runFiles function signature improves readability, this appears to be purely a formatting change unrelated to the main purpose of adding init(). Consider keeping such formatting changes in a separate commit or PR to maintain clearer git history and easier code review.
| runFiles: ( | |
| filenames: string[], | |
| files: Readonly<Record<string, string>> | |
| ) => Promise<ReplOutput[]>; | |
| runFiles: (filenames: string[], files: Readonly<Record<string, string>>) => Promise<ReplOutput[]>; |
app/terminal/README.md
Outdated
| * useRuntime() 内のuseEffectで呼び出されます。 | ||
| * ランタイムの初期化にコストがかかるものは、init()されたときにだけ初期化するようにします。 | ||
| * useRuntime() が複数回使われた場合はinitも複数回呼ばれます。 | ||
| * 初期化は非同期に行い、完了する前にreturnしてよいです。 |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] In the documentation, line 17 states "初期化は非同期に行い、完了する前にreturnしてよいです" (initialization is done asynchronously and may return before completion). This is accurate for the TypeScript runtime's init() implementation which starts async initialization without awaiting. However, consider clarifying that the ready flag should be checked before using the runtime to ensure initialization has completed.
| * 初期化は非同期に行い、完了する前にreturnしてよいです。 | |
| * 初期化は非同期に行い、完了する前にreturnしてよいです。 | |
| * そのため、ランタイムを利用する前に `ready` フラグが `true` であることを必ず確認してください。 |
| const { init } = runtime; | ||
| useEffect(() => { | ||
| init?.(); | ||
| }, [init]); |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The useEffect has init in its dependency array, but init is a callback that may change on every render depending on its dependencies. This could cause unnecessary re-initialization. Since the README documentation states that "useRuntime() が複数回使われた場合はinitも複数回呼ばれます", this behavior might be intentional, but consider whether this effect should only run once by using an empty dependency array instead.
| }, [init]); | |
| }, [language]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
close #106